Blocking: TOCTOU race on reservation name uniqueness. This check reads self.reservations before propose(); two concurrent create_reservation calls for the same name can both pass it before either commits. apply_operation's ReservationCreate arm never re-checks for an existing name and unconditionally pushes, so duplicates can land. Worse, ReservationDelete's apply arm uses retain(|r| r.name != *name), which would then remove both duplicates on a single delete. The uniqueness invariant needs to be enforced inside apply_operation — the single serialized point all replicas agree on — not just at propose time. Once fixed, a regression test spawning two same-name creates and asserting only one survives would pin down the invariant.

Fixed. apply_operation now dedupes ReservationCreate at the serialized apply point; ClientResponse.reservation_created reports whether it applied, and the RPC returns AlreadyExists otherwise. Added concurrent_reservation_create_keeps_single_entry test.

Blocking: no_hold_jobs doesn't actually let jobs be rescheduled. Skipping hold_jobs_for_deleted_reservation here (and in purge_expired_reservations) never clears job.spec.reservation — no code path does. reservation_block() unconditionally returns Some(PendingReason::Reservation) when the named reservation no longer exists, and pending_jobs() filters those out entirely. So jobs from a no_hold_jobs-deleted reservation are still permanently excluded from scheduling — functionally identical to being held, just without the Held label. A minimal regression test (extend purge_expired_holds_pending_reservation_jobs below at line ~5363): create a reservation with no_hold_jobs, submit a job against it, delete/expire it, assert the job is still eligible via pending_jobs().

Fixed. Delete/purge now clears job.spec.reservation via detach_jobs_from_deleted_reservation_jobs in the WAL apply path, so reservation_block() no longer filters those jobs. Added no_hold_jobs_reservation_delete_does_not_permanently_block_job and purge variant tests.

Suggestion: this fences on any node being reservation-blocked, even if 99 other free nodes would satisfy the job, because it continues before the more accurate Resources/NodeDown/Priority classification runs. It's diagnostics-only (actual placement uses prospective_overlap independently in spur-sched), but in any cluster with an active/upcoming reservation covering even one node, unrelated pending jobs can be mislabeled ReqNodeNotAvail/ReservedMaintenance in squeue/scontrol when the real reason is unrelated.

Fixed. Fence reasons are only set when all candidate nodes are reservation-blocked (job_candidate_node_names). Diagnostics-only path; scheduling still uses prospective_overlap independently.

Suggestion (pre-existing gap, not introduced by this PR): once requeue_count >= MAX_REQUEUE, the job returns Ok(()) and stays in JobState::Preempted forever — neither is_terminal() nor is_active() — so it never completes, gets cancelled, or gets picked up by any further scheduler hook. Consider transitioning to Cancelled with a distinguishing exit reason once retries are exhausted.

Fixed. Exhausted requeues now transition to held Pending with JobHoldMaxRequeue (configurable via max_batch_requeue in spur.conf) instead of staying in Preempted forever. Admin can release with scontrol release.

Nit: classifying gRPC status by substring-matching anyhow::Error::to_string() is fragile — any future rewording of an error message (including ones from hostlist::expand()/normalize_node_list(), which this function doesn't control) silently changes the client-facing status code. Consider a typed error enum or downcast_ref at the ClusterManager boundary instead.

Fixed. Introduced typed ReservationError; reservation_rpc_status maps variants to gRPC codes directly — no to_string() substring matching.

Nit: reservation_rpc_status returns a bare Status, not a Result<_, E>, so #[allow(clippy::result_large_err)] can never fire here — looks like a copy-paste leftover, safe to remove.

Fixed — removed the unused allow.

Suggestion: this duplicates prospective_overlap in spur-core/src/reservation.rs — both implement "does [start, start+duration) intersect a reservation window this job isn't authorized for", just parameterized differently (now vs. an arbitrary future start). Consider generalizing prospective_overlap to take an explicit at: DateTime<Utc> and having this call it, so future changes to the authorization rule can't drift between the two call sites.

Fixed. Added prospective_overlap_at(job, res, node, at, duration) in spur-core; backfill calls it so the authorization rule lives in one place.

Suggestion: reservation = 52 is grouped under the // Array jobs comment above array_job_id/array_task_id, but it isn't array-related. Consider splitting into its own // Reservation comment line for readability — the field number itself (52) is fine as-is.

Fixed — moved under its own // Reservation comment, field number unchanged.

Test coverage: nearby tests (apply_reservation_create_update_delete, snapshot_and_restore) prove CRUD round-trips and snapshot/restore work, but none proves durability across an actual restart via WAL replay, which is this PR's headline claim. No reservation test drops a ClusterManager and starts a new one against the same on-disk raft log directory to assert the reservation reappears from replay (as opposed to a hand-invoked snapshot restore into a fresh dir2). Worth adding a reservation_survives_wal_replay test reusing this same TempDir/test_cluster scaffolding but restarting against dir.path().

Added reservation_survives_wal_replay: drops ClusterManager, restarts against the same Raft dir, asserts reservation survives replay.

Test coverage: only the default (holding) path is tested here. A minimal no_hold_jobs_reservation_delete_does_not_permanently_block_job test — create a reservation with no_hold_jobs set, submit a job against it, delete/expire the reservation, assert the job is still eligible via pending_jobs() — would catch the gap flagged above on delete_reservation/purge_expired_reservations.

Added no_hold_jobs_reservation_delete_does_not_permanently_block_job and purge variant — create with no_hold_jobs, submit job, delete/expire, assert job is still in pending_jobs().